Skip to content

fix: harden IPC limits and secure file handling#377

Open
SUJALMU2004 wants to merge 1 commit into
browser-use:mainfrom
SUJALMU2004:harden-ipc-security
Open

fix: harden IPC limits and secure file handling#377
SUJALMU2004 wants to merge 1 commit into
browser-use:mainfrom
SUJALMU2004:harden-ipc-security

Conversation

@SUJALMU2004
Copy link
Copy Markdown

@SUJALMU2004 SUJALMU2004 commented May 18, 2026

This PR introduces targeted, non-destructive hardening improvements to the daemon's IPC and file handling architecture.

During load testing and security review of the local daemon setup, I identified a functional limitation with large CDP payloads, a potential ToCToU socket creation race, and a CWE-61 symlink vulnerability in the default /tmp logging mechanism.

1. Fix IPC Payload Limit (Architecture & Functional)

The Root Cause:
asyncio.start_unix_server limits the reader.readline() buffer to 64KB by default. If an agent injects a large JavaScript library via helpers.js() or sends a large DOM CDP payload, the daemon silently terminates the connection with a LimitOverrunError.
The Fix:
I explicitly passed limit=1024 * 1024 * 16 (16MB) to the Unix socket initialization to natively support arbitrarily large AI-generated scripts. (Note: Windows TCP loopback is intentionally left at 64KB to prevent unauthenticated local clients from forcing large pre-auth memory allocations).

2. Prevent Unix Socket Denial of Service (Robustness)

The Root Cause:
The daemon cleans up stale sockets using if os.path.exists(path): os.unlink(path). Because os.path.exists() resolves symlinks, it returns False if the path is a broken symlink. If a broken symlink exists at the socket path, the unlink is bypassed, and the bind() call fatally crashes with Address already in use.
The Fix:
Refactored socket cleanup to use the standard, race-free EAFP pattern: try: os.unlink(path) except OSError: pass.

3. Patch Log/PID Symlink Vulnerability (Security - CWE-61)

The Root Cause:
daemon.py created the LOG and PID files directly in /tmp. Because /tmp is a world-writable shared directory on POSIX systems, a malicious local user could execute a symlink attack (CWE-61) by pre-creating /tmp/bu-default.log as a symlink pointing to another user's sensitive file (e.g., ~/.bashrc). The daemon would blindly follow the symlink and overwrite the target file.
The Fix:
Introduced a safe_open_write() helper utilizing the low-level os.open syscall with the os.O_NOFOLLOW flag to strictly reject symlink traversal at the kernel level (falling back gracefully to standard behavior on Windows).


Summary by cubic

Hardened the daemon’s IPC and file handling for stability and security. Supports large Unix socket payloads and blocks symlink-based file overwrites.

  • Bug Fixes

    • Raised Unix socket read limit to 16MB to handle large CDP/JS payloads; kept Windows TCP loopback at 64KB to reduce pre-auth DoS risk.
    • Switched socket cleanup to EAFP unlink (try/except OSError), fixing crashes when a broken symlink exists.
  • Security

    • Added safe_open_write() using os.O_NOFOLLOW and 0600 perms; applied to LOG and PID in /tmp to prevent CWE-61 symlink overwrites.
    • Startup now fails closed if a symlink is detected; logging uses the safe appender.

Written for commit 1e59cc8. Summary will update on new commits. Review in cubic

- Increased Unix socket server limit to 16MB to allow agents to inject large JS/HTML payloads without crashing.
- Maintained Windows TCP loopback at 64KB limit to prevent pre-auth local connection DoS.
- Switched socket unlink to use EAFP (try/except OSError) instead of os.path.exists to fix a crash caused by broken symlinks.
- Added a safe_open_write helper with O_NOFOLLOW and applied it to PID/LOG files to patch a CWE-61 symlink overwrite vulnerability in shared /tmp directories.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant